feat:get_ranges by d-v-b · Pull Request #3925 · zarr-developers/zarr-python

d-v-b · 2026-04-24T19:06:38Z

this PR adds a get_ranges protocol for stores. the protocol defines the shape of a function that fetches multiple byte ranges within the same stored object. The purpose is to define a method stores can opt into if they offer an efficient way to fetch multiple byte ranges from the same object, which would be immediately useful for the sharding codec. The protocol looks like this:

async def get_ranges(
    key: str,
    byte_ranges: Iterable[ByteRequest | None],
    *,
    prototype: BufferPrototype,
) -> AsyncIterator[Sequence[tuple[int, Buffer | None]]]:

the return type is an async iterator over sequences, where each sequence is the result of an IO operation the store implemented. this provides some observability to the caller about the actual coalescing, if any, that occurred. Results are returned in computed order, so the inner result type is tuple[int, Buffer | None], where the int is the index into the input byte_ranges for that result.

Only byte range requests that declare an explicit interval (RangeByteRequest) are coalesced. Any other byte range, or None, results in no coalescing and so the ranges will be fetched separately. I assume here that we do not care about coalescing overlapping suffix or prefix range requests, but we could add support for that if we need to.

In addition to this protocol, there's a freestanding function that takes:

a f(byte range) -> Awaitable[Buffer] function (which we would generate by combining Store.get with functools.partial)
an iterable of byte range requests
options (max gap bytes, max total bytes, etc)

this function contains basic byte range coalescing logic, and it can be re-used for multiple stores. This is a non-abstract-base-class alternative to a default implementation on an abc.

That freestanding function is used to implement get_ranges on the FsspecStore. This is probably not useful for local- or memory-backed storage, but is useful for remote storage. The actual implementation is lightweight:

    async def get_ranges(
        self,
        key: str,
        byte_ranges: Iterable[ByteRequest | None],
        *,
        prototype: BufferPrototype,
    ) -> AsyncIterator[Sequence[tuple[int, Buffer | None]]]:
        """Read many byte ranges from ``key``, coalescing nearby ranges and fetching concurrently.

        See :class:`zarr.storage._protocols.SupportsGetRanges` for the contract and
        :func:`zarr.core._coalesce.coalesced_get` for the full semantics.
        """
        fetch = partial(self.get, key, prototype)
        async for group in coalesced_get(fetch, byte_ranges, options=self.coalesce_options):
            yield group

cc @aldenks, the idea here is to build a basis for your range coalescing work for the sharding codec
cc @kylebarron, would love your feedback on this design.

related issues/PRs:

#1758
#3004

…sced_get

…ed_get

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Split the overlong first line into a short numpydoc summary plus an extended description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

After the input split at the top of coalesced_get, merged groups only ever contain RangeByteRequest members. Replace the per-element isinstance filters (and the defensive ``else 0`` sort-key branch) with a single assertion at the top of the merged-group block and direct attribute access. Also remove the unreachable ``if total == 0: return`` guard (``indexed`` is non-empty by construction once we pass the earlier guard). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Exercise the ``kind == "missing"`` branch in the uncoalescable single-fetch arm for Offset/Suffix/None inputs, which was not hit by existing tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two related correctness issues in coalesced_get's drain loop: 1. When the consumer breaks out of the async-for (early exit), the generator's finally block only awaited in-flight tasks rather than cancelling them. That wasted I/O. Cancel first, then gather. 2. The drain loop waited on completion_queue for ``total`` entries, but after a "missing" or "error" we cancel pending tasks -- and cancelled tasks never enqueue a completion. With max_concurrency > 1 this could hang. Rework the drain loop to break out immediately on the first miss/error; the finally block handles cleanup. The new structure also collapses the redundant miss/error branches and removes the now-unused ``total``/``drained``/``stopped`` bookkeeping. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Exercises the concurrent path where a missing key is observed while other fetches are still in flight. Uses an asyncio.Event to gate late arrivals until after the miss has been processed, giving the drain loop an opportunity to observe and discard post-stop completions, and verifies the iterator terminates cleanly without hanging or raising. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Drives many slow ranges with a small max_concurrency, breaks out of the async-for after the first yield, and verifies that at least one still-running fetch was cancelled rather than being left to run to completion. Cancellation is observed via a counter in the fetch's CancelledError branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coalesced_get is implemented as an async generator (uses yield) and callers need access to aclose() to drive its finally block deterministically. Declaring the return type as AsyncGenerator instead of AsyncIterator exposes aclose()/asend()/athrow() through the type system, matches the runtime object, and lets consumers (e.g. the consumer-break test) avoid type-ignore escape hatches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

pyproject asyncio_mode=auto already covers async test dispatch; the explicit pytestmark was a vestige. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Used 0000.feature.md as a placeholder; rename to {pr-number}.feature.md once the PR is opened. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The SupportsGetRanges protocol is private; a user-facing release note shouldn't advertise it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

d-v-b · 2026-04-24T19:17:52Z

for context, we do already have a get_partial_values method, but it returns eagerly (no streaming), and it handles fetching from multiple keys. get_ranges only targets a single key, and it supports streaming. This is IMO a more targeted and performance-friendly design, and it's narrowly what the sharding codec needs for IO against a single shard.

The min_deps CI job pins fsspec to 2023.10.0, which predates AsyncFileSystemWrapper. Wrapping a sync MemoryFileSystem fails there at fixture setup. Guard the affected tests with the same skipif pattern already used in test_fsspec.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

codecov · 2026-04-24T19:29:08Z

Codecov Report

❌ Patch coverage is 96.15385% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.30%. Comparing base (eac9c86) to head (63da755).

Files with missing lines	Patch %	Lines
src/zarr/storage/_wrapper.py	80.00%	2 Missing ⚠️
src/zarr/core/_coalesce.py	98.38%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3925      +/-   ##
==========================================
+ Coverage   93.28%   93.30%   +0.01%     
==========================================
  Files          87       88       +1     
  Lines       11745    11823      +78     
==========================================
+ Hits        10956    11031      +75     
- Misses        789      792       +3

Files with missing lines	Coverage Δ
src/zarr/abc/store.py	`96.42% <100.00%> (+0.13%)`	⬆️
src/zarr/core/_coalesce.py	`98.38% <98.38%> (ø)`
src/zarr/storage/_wrapper.py	`97.80% <80.00%> (-2.20%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

kylebarron · 2026-04-24T19:55:48Z

I think making it iterable adds complexity and adds confusion to whether request coalescing is expected to be applied here or not.

If you have requests:

0-1000
1000-2000
2000-3000
100_000 - 100_005

Then of course we should coalesce all the first 3 requests into one. But then the async iterable implies that we might want to use one of the first responses before the last one arrives... but the last request would've arrived first.

Either, if you really want the response type to be async iterable, the responses should probably have an index for which of the input requests it's associated to.

But I think it would be much simpler to take in a sequence of byte ranges and return a sequence of results. Just like object-store/obstore/obspec.

d-v-b · 2026-04-24T20:03:58Z

If you have requests:

* 0-1000

* 1000-2000

* 2000-3000

* 100_000 - 100_005

In the design in this PR we are iterating over IO calls the reader actually did, which is less than or equal to the number of byte ranges requested. So, assuming the first three are fused, we would either see:

yields ((0, <payload>), (1, <payload>), (2, <payload>))
yields ((3, <payload>),)

or

yields ((3, <payload>),)
yields ((0, <payload>), (1, <payload>), (2, <payload>))

Which, for sharding is useful -- you can start decoding chunks immediately while you wait for the rest of the sub-chunks to come in. Does that make sense? or am I misunderstanding something.

maxrjones

Nice, I think this is well-implemented and tested. I have one correctness issue and a few comments.

While I don't think this is the responsibility of this PR, I still want to register my general dissatisfaction with our API design that this PR inherits and extends, in particular with mixing ABC and protocol-based abstraction mechanisms. The issue here is that the new methods are not available in wrapper stores such as LoggingStore. Let's continue to work on a better architecture for the storage API.

maxrjones · 2026-05-11T17:54:17Z

+    # Launch all work as tasks. The semaphore bounds actual I/O concurrency.
+    tasks: set[asyncio.Task[None]] = set()
+    for group in groups:
+        tasks.add(asyncio.create_task(_fetch_group(ctx, group)))


Suggested change

tasks.add(asyncio.create_task(_fetch_group(ctx, group)))

# A one-member "group" is a RangeByteRequest that did not merge with a

# neighbor; route it through `_fetch_single` so it skips the redundant

# slice-by-zero in `_fetch_group`.

if len(group) == 1:

idx, single = group[0]

tasks.add(asyncio.create_task(_fetch_single(ctx, idx, single)))

else:

tasks.add(asyncio.create_task(_fetch_group(ctx, group)))```

@maxrjones, that's a bit of a micro-optimization, but if you want to make that logic change, I suggest pushing that into _fetch_group directly, where it will check the length of the list, and defer to _fetch_single for a singleton list, leaving the code here cleaner.

good idea! I applied this in 5731be6

maxrjones · 2026-05-11T17:55:07Z

        self.fs = fs
        self.path = path
        self.allowed_exceptions = allowed_exceptions
+        self.coalesce_options = coalesce_options


Suggested change

self.coalesce_options = coalesce_options

# Copy so per-instance mutation of `self.coalesce_options` does not

# leak into the module-level `DEFAULT_COALESCE_OPTIONS` singleton

# (or into other stores constructed without an explicit kwarg).

self.coalesce_options: CoalesceOptions = (

DEFAULT_COALESCE_OPTIONS.copy() if coalesce_options is None else coalesce_options.copy()

)```

coalesce_options should be immutable (or at least typed as such), so that any erroneous attempt to mutate them should fail (either during static analysis or at runtime), so this should be unnecessary.

To aid in this effort, I suggest considering a frozen dataclass rather than a TypedDict for CoalesceOptions.

To aid in this effort, I suggest considering a frozen dataclass rather than a TypedDict for CoalesceOptions.

we can annotate the fields as ReadOnly

Could do, but in this case, I would lean towards a kw-only, frozen dataclass for the following reasons:

no real motivation for using a dict here (we're not marshaling between JSON)

we can set defaults for every field, thus eliminating the need for a separate default instance (simply using CoalesceOptions() as the param default would do the trick)

allows specifying only what we want to override rather than having to specify everything (e.g., passing CoalesceOptions(max_concurrency=20) will use the default values for the other fields). Of course, we could mark the TypedDict as total=False, but that would require a bit more code to fill in the default values for the user (not much, but a bit less convenient than using a dataclass, IMO).

I've added a suggestion to the CoalesceOptions definition.

maxrjones · 2026-05-11T18:08:36Z

+# src/zarr/storage/_protocols.py
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Protocol, runtime_checkable
+
+if TYPE_CHECKING:
+    from collections.abc import AsyncIterator, Sequence
+
+    from zarr.abc.store import ByteRequest
+    from zarr.core.buffer import Buffer, BufferPrototype
+
+
+@runtime_checkable
+class SupportsGetRanges(Protocol):
+    """Stores that satisfy this protocol can efficiently read many byte ranges
+    from a single key in a single call, typically via coalescing and concurrent fetch.
+
+    Private / unstable. Shape may change before being made public.
+    """
+
+    def get_ranges(
+        self,
+        key: str,
+        byte_ranges: Sequence[ByteRequest | None],
+        *,
+        prototype: BufferPrototype,
+    ) -> AsyncIterator[Sequence[tuple[int, Buffer | None]]]:
+        """Read many byte ranges from `key`.
+
+        Each yield corresponds to one underlying I/O operation.
+
+        See `zarr.core._coalesce.coalesced_get` for full semantics.
+        """
+        ...


we should either put this in zarr/abc/store.py with the other protocols or more the other protocols here. It's confusing to have this separate from similar protocols.

maxrjones · 2026-05-11T18:13:47Z

+    """Stores that satisfy this protocol can efficiently read many byte ranges
+    from a single key in a single call, typically via coalescing and concurrent fetch.
+
+    Private / unstable. Shape may change before being made public.


given that this is intended to be private/unstable, should the methods be prefixed with a _ and the addition not included in the changelog as a feature release?

Co-authored-by: Max Jones <14077947+maxrjones@users.noreply.github.com>

d-v-b · 2026-05-11T21:24:33Z

While I don't think this is the responsibility of this PR, I still want to register my general dissatisfaction with our API design that this PR inherits and extends, in particular with mixing ABC and protocol-based abstraction mechanisms. The issue here is that the new methods are not available in wrapper stores such as LoggingStore. Let's continue to work on a better architecture for the storage API.

I don't mind adding this method to the Store abc! I think I opted for the protocol initially because I assumed it would be easier to get a new protocol added than change the base Store class. But if we are all on board with changes to the base Store class, then IMO that's simpler.

chuckwondo · 2026-05-11T23:09:43Z

        self.fs = fs
        self.path = path
        self.allowed_exceptions = allowed_exceptions
+        self.coalesce_options = coalesce_options


I've added a suggestion to the CoalesceOptions definition.

chuckwondo · 2026-05-11T23:12:53Z

+DEFAULT_COALESCE_OPTIONS: CoalesceOptions = {
+    "max_gap_bytes": 1 << 20,  # 1 MiB
+    "max_coalesced_bytes": 16 << 20,  # 16 MiB
+    "max_concurrency": 10,


Suggested change

DEFAULT_COALESCE_OPTIONS: CoalesceOptions = {

"max_gap_bytes": 1 << 20, # 1 MiB

"max_coalesced_bytes": 16 << 20, # 16 MiB

"max_concurrency": 10,

chuckwondo · 2026-05-11T23:23:17Z

+    # Launch all work as tasks. The semaphore bounds actual I/O concurrency.
+    tasks: set[asyncio.Task[None]] = set()
+    for group in groups:
+        tasks.add(asyncio.create_task(_fetch_group(ctx, group)))


@maxrjones, that's a bit of a micro-optimization, but if you want to make that logic change, I suggest pushing that into _fetch_group directly, where it will check the length of the list, and defer to _fetch_single for a singleton list, leaving the code here cleaner.

…ileNotFoundError when get yields None

chuckwondo · 2026-05-12T16:04:33Z

+    max_gap_bytes: int | None = None,
+    max_coalesced_bytes: int | None = None,


Why are you not specifying the defaults here? Doing so would make it obvious in and IDE what the defaults are, without having to go look for them. It also allows you to simplify the function by eliminating the is None checks.

I think that's a good idea. In order to preserve the None sentinel behavior in an ergonomic way, we need to bring back a typeddict that models these kwargs so callers can construct such a dict and omit any keys that should take the default value.

I don't think a typeddict is necessary. I'm fine with the individual kwargs, but I would prefer to eliminate the Nones and defaulting to the default values explicitly.

The problem with a typeddict in this case is that it makes it more awkward_ to deal with default values, or at least more verbose.

the nones are gone, and the defaults are set in just the store get_ranges method. This is less convenient for callers of the internal routines, but IMO balances convenience for people calling the store method with avoiding redundant default parameters at different levels of detail. no need for typeddicts

chuckwondo · 2026-05-12T16:07:16Z

+    max_concurrency: int | None = None,
+    max_gap_bytes: int | None = None,
+    max_coalesced_bytes: int | None = None,


Again, I suggest using the defaults. I realize it may get a bit redundant to repeat the defaults everywhere, but doing so is more pleasant for the user experience.

d-v-b · 2026-05-12T16:29:49Z

@maxrjones and @chuckwondo thanks for the great feedback, I rolled a lot of your points into a series of changes, summarized in this bulleted list:

the protocol is gone and we have a new method on the store abc with a default implementation. get_ranges should work for all stores now
the queue is gone, replaced with iteration over as_completed
we abort as soon as any of the get requests returns None, and raise FileNotFoundError. We can't handle the results from an inconsistent read, so an exception makes sense here.
The typeddict is gone, and we just use plain keyword arguments. None is a sentinel value that means "look up the default". When we set up a call site for get_ranges in the sharding codec operations, we will need to ensure that these parameters are passed in (e.g., from the per-array configuration).

...I see chunk has more suggestions, so the above list might already be stale xD

Co-authored-by: Chuck Daniels <cjdaniels4@gmail.com>

…into feat/get-many

chuckwondo

We're getting very close!

chuckwondo · 2026-05-12T20:35:07Z

+        max_concurrency: int = 10,
+        max_gap_bytes: int = 1 << 20,
+        max_coalesced_bytes: int = 16 << 20,


Nice. I think this makes good sense. Perhaps just add some comments for clarity:

Suggested change

max_concurrency: int = 10,

max_gap_bytes: int = 1 << 20,

max_coalesced_bytes: int = 16 << 20,

max_concurrency: int = 10,

max_gap_bytes: int = 1 << 20, # 1 MiB

max_coalesced_bytes: int = 16 << 20, # 16 MiB

chuckwondo · 2026-05-12T22:02:29Z

+    max_concurrency: int,
+    max_gap_bytes: int,
+    max_coalesced_bytes: int,
+) -> AsyncIterator[Sequence[tuple[int, Buffer | None]]]:


So we don't have to cast in order to invoke aclose():

Suggested change

) -> AsyncIterator[Sequence[tuple[int, Buffer | None]]]:

) -> AsyncGenerator[Sequence[tuple[int, Buffer | None]]]:

chuckwondo · 2026-05-12T22:04:04Z

+        # Unwrap: prefer GeneratorExit, then a single inner exception, otherwise raise group.
+        for exc in eg.exceptions:
+            if isinstance(exc, GeneratorExit):
+                raise exc from None
+        if len(eg.exceptions) == 1:
+            raise eg.exceptions[0] from None
+        raise


We do NOT want to reraise GeneratorExit:

Suggested change

# Unwrap: prefer GeneratorExit, then a single inner exception, otherwise raise group.

for exc in eg.exceptions:

if isinstance(exc, GeneratorExit):

raise exc from None

if len(eg.exceptions) == 1:

raise eg.exceptions[0] from None

raise

# Unwrap: prefer a single inner exception, otherwise raise group.

if subgroup := eg.subgroup(lambda e: not isinstance(e, GeneratorExit)):

e = subgroup.exceptions[0] if len(subgroup.exceptions) == 1 else subgroup

raise e from None

chuckwondo · 2026-05-12T22:06:40Z

+    # Explicitly close the generator so its finally block runs (cancelling
+    # in-flight tasks) before we make assertions. The narrow AsyncIterator
+    # return type does not expose `.aclose()`, but the runtime object is an
+    # async generator and supports it.
+    await cast("AsyncGenerator[Any, None]", agen).aclose()


Suggested change

# Explicitly close the generator so its finally block runs (cancelling

# in-flight tasks) before we make assertions. The narrow AsyncIterator

# return type does not expose `.aclose()`, but the runtime object is an

# async generator and supports it.

await cast("AsyncGenerator[Any, None]", agen).aclose()

# Explicitly close the generator so its finally block runs (cancelling

# in-flight tasks) before we make assertions.

await agen.aclose()

chuckwondo · 2026-05-12T22:08:38Z

+    assert cancelled_calls >= 1
+    assert completed_calls >= 1


Suggested change

assert cancelled_calls >= 1

assert completed_calls >= 1

assert completed_calls >= 1

assert cancelled_calls == len(ranges) - completed_calls

d-v-b and others added 24 commits April 24, 2026 20:19

feat(core): add _coalesce module skeleton with CoalesceOptions and stub

d007c64

test(core): add failing tests for coalesced_get basic cases

abac6d3

feat(core): implement coalesced_get for basic sequential cases

f65018a

test(core): cover Offset/Suffix/None and mixed-cluster cases in coale…

9e1f1d2

…sced_get

feat(core): run coalesced fetches concurrently under max_concurrency

cd5097b

test(core): cover key-missing (start/mid) and fetch-raises in coalesc…

3a85488

…ed_get

test(core): cover max_coalesced_bytes cap in coalesced_get

4553523

test(core): add coverage-invariant property test for coalesced_get

162dd6d

test(core): drop unused HEAVY_MERGE/NO_MERGE constants

401e28b

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs(core): shorten coalesced_get docstring summary line

913928c

Split the overlong first line into a short numpydoc summary plus an extended description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test(core): cover key-missing on uncoalescable input

b2ec638

Exercise the ``kind == "missing"`` branch in the uncoalescable single-fetch arm for Offset/Suffix/None inputs, which was not hit by existing tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor(test): drop redundant pytestmark asyncio

865baf0

pyproject asyncio_mode=auto already covers async test dispatch; the explicit pytestmark was a vestige. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

feat(storage): add private SupportsGetRanges protocol

17d9f75

test(storage): add failing tests for FsspecStore.get_ranges

3ab711d

feat(storage): add FsspecStore.get_ranges and coalesce_options kwarg

913be10

test(storage): add SupportsGetRanges conformance tests

0328e01

chore: add towncrier fragment for get_ranges

e7432c5

Used 0000.feature.md as a placeholder; rename to {pr-number}.feature.md once the PR is opened. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs: drop private-symbol mention from get_ranges changelog

349bd9c

The SupportsGetRanges protocol is private; a user-facing release note shouldn't advertise it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test: refactor tests

79e9927

d-v-b mentioned this pull request Apr 24, 2026

Optimize partial shard reads #3004

Open

6 tasks

chuckwondo reviewed May 8, 2026

View reviewed changes

Comment thread src/zarr/storage/_protocols.py Outdated

d-v-b added 4 commits May 8, 2026 13:31

Merge branch 'main' into feat/get-many

a997792

refactor(coalesce): split coalescing from coalesced get

17f8d64

refactor(coalesce): use sequence instead of iterable

5eb25bc

Merge branch 'main' into feat/get-many

249ea2c

ilan-gold mentioned this pull request May 11, 2026

perf: phased codecpipeline #3885

Open

maxrjones reviewed May 11, 2026

View reviewed changes

d-v-b and others added 2 commits May 11, 2026 22:26

Update src/zarr/storage/_fsspec.py

391985a

Co-authored-by: Max Jones <14077947+maxrjones@users.noreply.github.com>

Merge branch 'main' into feat/get-many

e56fc01

chuckwondo reviewed May 11, 2026

View reviewed changes

Comment thread src/zarr/core/_coalesce.py Outdated

chuckwondo suggested changes May 12, 2026

View reviewed changes

d-v-b added 5 commits May 12, 2026 14:13

refactor: banish typeddict, we are fine with kwargs

5731be6

refactor: apply suggestions from code review

b96dd14

refactor: use drop queue in favor of as_completed; abort early with F…

cbb42e9

…ileNotFoundError when get yields None

refactor: define get_ranges on the store abc

7368206

Merge branch 'main' into feat/get-many

aad4e0e

chuckwondo suggested changes May 12, 2026

View reviewed changes

d-v-b and others added 10 commits May 12, 2026 18:30

Update src/zarr/core/_coalesce.py

76a3a9b

Co-authored-by: Chuck Daniels <cjdaniels4@gmail.com>

Apply suggestion from @chuckwondo

470ab80

Co-authored-by: Chuck Daniels <cjdaniels4@gmail.com>

Apply suggestion from @chuckwondo

236fe05

Co-authored-by: Chuck Daniels <cjdaniels4@gmail.com>

refactor: use function-scoped defaults

4d7a24e

Merge branch 'feat/get-many' of https://github.com/d-v-b/zarr-python …

c55ae5b

…into feat/get-many

fix: fix failing test

11e6c39

refactor: defaults in one place

83cc824

Merge branch 'main' into feat/get-many

ca6477a

hoist imports

c22eef6

Merge branch 'feat/get-many' of https://github.com/d-v-b/zarr-python …

63da755

…into feat/get-many

chuckwondo suggested changes May 12, 2026

View reviewed changes

-        tasks.add(asyncio.create_task(_fetch_group(ctx, group)))
+        # A one-member "group" is a RangeByteRequest that did not merge with a
+        # neighbor; route it through `_fetch_single` so it skips the redundant
+        # slice-by-zero in `_fetch_group`.
+        if len(group) == 1:
+            idx, single = group[0]
+            tasks.add(asyncio.create_task(_fetch_single(ctx, idx, single)))
+        else:
+            tasks.add(asyncio.create_task(_fetch_group(ctx, group)))```

-        self.coalesce_options = coalesce_options
+        # Copy so per-instance mutation of `self.coalesce_options` does not
+        # leak into the module-level `DEFAULT_COALESCE_OPTIONS` singleton
+        # (or into other stores constructed without an explicit kwarg).
+        self.coalesce_options: CoalesceOptions = (
+            DEFAULT_COALESCE_OPTIONS.copy() if coalesce_options is None else coalesce_options.copy()
+        )```

		max_gap_bytes: int \| None = None,
		max_coalesced_bytes: int \| None = None,

	) -> AsyncIterator[Sequence[tuple[int, Buffer \| None]]]:
	) -> AsyncGenerator[Sequence[tuple[int, Buffer \| None]]]:

Uh oh!

Conversation

d-v-b commented Apr 24, 2026

Uh oh!

d-v-b commented Apr 24, 2026

Uh oh!

codecov Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kylebarron commented Apr 24, 2026

Uh oh!

d-v-b commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

maxrjones left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chuckwondo May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-v-b commented May 11, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chuckwondo May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

d-v-b commented May 12, 2026

Uh oh!

chuckwondo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov Bot commented Apr 24, 2026 •

edited

Loading

d-v-b commented Apr 24, 2026 •

edited

Loading

chuckwondo May 11, 2026 •

edited

Loading

chuckwondo May 12, 2026 •

edited

Loading